Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): Add TotpInfo field to UserRecord #573

Merged
merged 14 commits into from
Nov 7, 2023
Merged

Conversation

pragatimodi
Copy link
Contributor

@pragatimodi pragatimodi commented Jul 24, 2023

Original PR based off master
This is based off dev branch.

@pragatimodi pragatimodi changed the base branch from master to dev July 24, 2023 20:58
auth/user_mgt.go Outdated Show resolved Hide resolved
Copy link

@Xiaoshouzi-gh Xiaoshouzi-gh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM.

Optional - To gradually improve the coding practice, we can start clean up while we touching the existing code. For example, we can start moving repeated string in test cases in constants.

auth/user_mgt.go Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Show resolved Hide resolved
@pragatimodi pragatimodi changed the title feat(auth): Add TotpInfo field to UserRecord feat(auth): Add TotpInfo field to UserRecord Jul 25, 2023
@lahirumaramba lahirumaramba added the release:stage Stage a release candidate label Jul 25, 2023
Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall! Thank you! Added a few more comments.

auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Show resolved Hide resolved
auth/user_mgt_test.go Show resolved Hide resolved
auth/user_mgt_test.go Outdated Show resolved Hide resolved
integration/auth/user_mgt_test.go Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
@davidbudnick
Copy link

Good Afternoon Team 👋🏻

Any status updates on getting this MR merged in?

Thanks!

auth/user_mgt.go Show resolved Hide resolved
@pragatimodi pragatimodi merged commit 8355839 into dev Nov 7, 2023
8 checks passed
@pragatimodi pragatimodi deleted the user-record-mfa branch November 7, 2023 21:53
@davidbudnick
Copy link

@pragatimodi 👋🏻 Thanks for pushing the changes to get this merged into the dev branch. Looking through the CONTRIBUTING page I did not see any details about how the release process works for this project.

Do you have any idea on when my team can see it pushed to a new version?

Thanks!

@msidd
Copy link

msidd commented Nov 10, 2023

@pragatimodi Can you please provide ETA on the next release of SDK that has this feature. A critical release is blocked due to this feature missing in the current version of SDK

@pragatimodi
Copy link
Contributor Author

@lahirumaramba Please provide an update on the estimated release for this?

@msidd
Copy link

msidd commented Nov 14, 2023

@lahirumaramba Can you please provide ETA for the next release, as i stated earlier a critical release is blocked due to this. We tried to reach out to GCP premium support if but they pointed us here

@lahirumaramba
Copy link
Member

Thanks for your patience folks! We will include this change in the planned upcoming release this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:stage Stage a release candidate release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants